Skip to content

Conversation

@renaudhartert-db
Copy link
Contributor

@renaudhartert-db renaudhartert-db commented Nov 9, 2025

What changes are proposed in this pull request?

This PR fixes a bug where the Databricks Java SDK was returning a generic DatabricksError instead of the specific TooManyRequests exception for HTTP 429 (Too Many Requests) responses.

Key Changes:

Removed hardcoded 429 handling (ApiErrors.java):

  • Previously, HTTP 429 responses were handled by a hardcoded check that returned a generic DatabricksError("TOO_MANY_REQUESTS", ...). This was likely added as a quick fix in past but resulted in bypassing the existing error mapping infrastructure that would have correctly returned a TooManyRequests instance. Now, all HTTP errors (including 429) flow through the ErrorMapper, which has the proper statusCode(429, TooManyRequests::new) mapping.

Refactored error parsing (ApiErrors.java):

  • parseApiError() now returns Optional<ApiErrorBody> to explicitly handle cases where the response body is null or empty.
  • When the response body is empty, getDatabricksError() passes an empty ApiErrorBody to the ErrorMapper, which correctly infers the error type based solely on the HTTP status code.
  • Extracted normalizeError() method to centralize logic for handling older API error formats (API v1.2 and SCIM errors).

Simplified error flow (ApiClient.java):

  • Removed the dual error handling path that passed both Response and Exception to getDatabricksError().
  • IOException during HTTP request execution is now immediately converted to DatabricksError("IO_ERROR", 523, exception) without retry logic.
  • Signature simplified from getDatabricksError(Response out, Exception error) to getDatabricksError(Response response).

How is this tested?

The change is covered by existing unit tests that have been adapted to verify that 429 error are properly mapped to the right exception.

@renaudhartert-db renaudhartert-db changed the title Correctly parse 429 errors Correctly return TooManyRequests when receiving HTTP 429 errors. Nov 9, 2025
@github-actions
Copy link

github-actions bot commented Nov 9, 2025

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/sdk-java

Inputs:

  • PR number: 547
  • Commit SHA: 2b5cc5da4a40944aa23ca5bd0f6bd89e81b706d3

Checks will be approved automatically on success.

Copy link
Contributor

@Divyansh-db Divyansh-db left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just had one doubt, otherwise LGTM

@renaudhartert-db renaudhartert-db added this pull request to the merge queue Nov 10, 2025
Merged via the queue into main with commit 237748f Nov 10, 2025
16 checks passed
@renaudhartert-db renaudhartert-db deleted the renaud-hartert_data/api-error branch November 10, 2025 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants